Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: add cross-platform perf reader implementation and fix type mismatch #1969

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: add cross-platform perf reader implementation and fix type mismatch #1969

wants to merge 4 commits into from

Conversation

Nikki371
Copy link

Issue

Errors encountered during local make build on non Linux System.

Although the expectation is to use docker images or virtual OS, thought this might help clear the obvious build issues.

# go.opentelemetry.io/auto/internal/pkg/process
internal/pkg/process/allocate.go:42:13: invalid operation: pagesize * nCPU (mismatched types uint64 and int)
internal/pkg/process/allocate.go:63:14: cannot use nCPU (variable of type int) as uint64 value in struct literal
make: *** [build] Error 1

CGO_ENABLED=0 go build -o otel-go-instrumentation ./cli/...
# go.opentelemetry.io/auto/internal/pkg/instrumentation/bpffs
internal/pkg/instrumentation/bpffs/bpfsfs_other.go:12:47: undefined: process.TargetDetails
internal/pkg/instrumentation/bpffs/bpfsfs_other.go:16:28: undefined: process.TargetDetails
internal/pkg/instrumentation/bpffs/bpfsfs_other.go:20:30: undefined: process.TargetDetails
make: *** [build] Error 1

CGO_ENABLED=0 go build -o otel-go-instrumentation ./cli/...
# go.opentelemetry.io/auto/internal/pkg/instrumentation/probe
internal/pkg/instrumentation/probe/probe.go:76:26: undefined: perf.Record
internal/pkg/instrumentation/probe/probe.go:78:24: undefined: perf.Reader
internal/pkg/instrumentation/probe/probe.go:236:23: undefined: perf.NewReader
internal/pkg/instrumentation/probe/probe.go:267:27: undefined: perf.ErrClosed
internal/pkg/instrumentation/probe/probe.go:321:27: undefined: perf.ErrClosed
internal/pkg/instrumentation/probe/probe.go:353:27: undefined: perf.ErrClosed
make: *** [build] Error 1

Environment

  • OS: Apple M2 (macOS : 15.3.1 (24D70))
  • Go Version: 1.24
  • Version: v.0.21.0

To Reproduce

Steps to reproduce the behavior:

  1. Clone opentelemetry-go-instrumentation to local
  2. run make build

Expected behavior

Expected make build to be successful on non linux systems too as the mock implementation is already present

Changes

  • Created platform-specific implementations for perf reader with Linux implementation using github.com/cilium/ebpf/perf and stub implementations for non-Linux platforms
  • Fixed type mismatches by updating process.TargetDetails to process.Info in bpffs package
  • Changed GetCPUCount return type from int to uint64 for consistency
  • Improved documentation for kernel lockdown mode constants
  • Updated import statements to use the new internal perf package

@Nikki371
fix: add cross-platform perf reader implementation and fix type misma…
3c1969f 
…tches

- Created platform-specific implementations for perf reader with Linux implementation
  using github.com/cilium/ebpf/perf and stub implementations for non-Linux platforms
- Fixed type mismatches by updating process.TargetDetails to process.Info in bpffs package
- Changed GetCPUCount return type from int to uint64 for consistency
- Improved documentation for kernel lockdown mode constants
- Updated import statements to use the new internal perf package
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Mar 11, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@Nikki371 Nikki371 marked this pull request as ready for review March 11, 2025 12:17
@Nikki371 Nikki371 requested a review from a team as a code owner March 11, 2025 12:17
RonFed
RonFed requested changes Mar 14, 2025
View reviewed changes
Copy link
Contributor

@RonFed RonFed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the perf change.
It broke because of cilium/ebpf#1650
From the cilium slack it looks like it broke other stuff as well.
I'm not sure if we need to create these stubs here or it should be handled upstream in the cilium library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RonFed RonFed RonFed requested changes

@MrAlias MrAlias MrAlias approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Nikki371 @MrAlias @RonFed